Skip to content
This repository has been archived by the owner on Nov 17, 2023. It is now read-only.

Add more gluon computation test on MKLDNN with memory operation(s)(slice, reshape etc.) #10764

Merged
merged 1 commit into from
May 1, 2018

Conversation

juliusshufan
Copy link
Contributor

@juliusshufan juliusshufan commented May 1, 2018

Description

This PR introduces more test cases to cover the gluon computation on MKLDNN with ndarray related memory operations, such as slice, reshape etc. I am also implementing more test cases covering more gluon computations involving MKLDNN (such as BN, pooling etc.), and will be reflected in standalone PR(s) accordingly.

Checklist

Essentials

Please feel free to remove inapplicable items for your PR.

  • The PR title starts with [MXNET-$JIRA_ID], where $JIRA_ID refers to the relevant JIRA issue created (except PRs with tiny changes)
  • Changes are complete (i.e. I finished coding on this PR)
  • All changes have test coverage:
  • Unit tests are added for small changes to verify correctness (e.g. adding a new operator)
  • Nightly tests are added for complicated/long-running ones (e.g. changing distributed kvstore)
  • Build tests will be added for build configuration changes (e.g. adding a new build option with NCCL)
  • Code is well-documented:
  • For user-facing API changes, API doc string has been updated.
  • For new C++ functions in header files, their functionalities and arguments are documented.
  • For new examples, README.md is added to explain the what the example does, the source of the dataset, expected performance on test set and reference to the original paper if applicable
  • Check the API doc at http://mxnet-ci-doc.s3-accelerate.dualstack.amazonaws.com/PR-$PR_ID/$BUILD_ID/index.html
  • To the my best knowledge, examples are either not affected by this change, or have been fixed to be compatible with this change

Changes

All the changes is reflected by tests/python/mkl/test_mkldnn.py

Comments

  1. For the correctness check on gluon computation, it follows the design used by tests/python/unittest/test_gluon.py, and therefore, the helper functions defined in tests/python/unitest/common.py is also used.
  2. A minor change on the existing test case in same source file is just addressing the additional module import supporting the newly added test cases.

@juliusshufan juliusshufan changed the title Add more gluon computation on MKLDNN with memory operation(s)(slice, reshape etc.) Add more gluon computation test on MKLDNN with memory operation(s)(slice, reshape etc.) May 1, 2018
@piiswrong piiswrong merged commit 1f42f2c into apache:master May 1, 2018
@ashokei
Copy link
Contributor

ashokei commented May 1, 2018

@juliusshufan @piiswrong we cannot import mxnet in this test, as one of the unittests test if we can load mxnet properly. May be we could move that test to separate file.

@ashokei
Copy link
Contributor

ashokei commented May 1, 2018

@piiswrong made changes here #10591

@juliusshufan
Copy link
Contributor Author

@ashokei Hi Ashok, if I hope keep these cases in same source file, I need to import the mxnet module for each case separately similar to the existing cases, right? If this is the right understanding, I'll make changes in next PR. Thanks.

@ashokei
Copy link
Contributor

ashokei commented May 2, 2018

I think it is good idea to move the test_mkldnn_install to separate file, i made changes in #10591 . Your changes are ok.

@pengzhao-intel
Copy link
Contributor

Good job @juliusshufan !
It's really what we need to verify the quality of new MKL-DNN backend.
In the future, we will add more test cases for the CPU.

Thanks, @piiswrong for the review and merging.

@zheng-da
Copy link
Contributor

zheng-da commented May 2, 2018

to be honest, i was a little surprised that the test is passed. i expect mkldnn conv doesn't work on an ndarray generated as a view of a MKLDNN NDArray (i.e., a slice from the mkldnn conv output). maybe we should take a little closer look at the C++ code how it works.

@ashokei
Copy link
Contributor

ashokei commented May 2, 2018

I notice these tests seem to just use nchw.

@juliusshufan i would consider using tensor shapes of (32,3,224,224), this would trigger mkldnn layout conversions.

@juliusshufan
Copy link
Contributor Author

@ashokei Thanks for comments, I might take a trial and make amendment.

anirudh2290 pushed a commit to anirudh2290/mxnet that referenced this pull request May 7, 2018
rahul003 pushed a commit to rahul003/mxnet that referenced this pull request Jun 4, 2018
zheng-da pushed a commit to zheng-da/incubator-mxnet that referenced this pull request Jun 8, 2018
zheng-da pushed a commit to zheng-da/incubator-mxnet that referenced this pull request Jun 28, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants